Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Also push MAJOR and MAJOR.MINOR tags when releasing #468

Merged
merged 2 commits into from
Dec 7, 2021
Merged

Also push MAJOR and MAJOR.MINOR tags when releasing #468

merged 2 commits into from
Dec 7, 2021

Conversation

pbusko
Copy link
Contributor

@pbusko pbusko commented Nov 17, 2021 •
edited
Loading

Summary

As decided in the RFC 39

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@pbusko pbusko requested a review from a team November 17, 2021 15:45
@dmikusa
Copy link
Contributor

dmikusa commented Nov 17, 2021

Thanks for the PR, we'll try to take a look at this shortly. Just an FYI, changes to the pipeline require additional review and testing so they can take a little longer to be merged.

Quick note, you need to run go generate because this PR modifies shell scripts. Running go generate will update the scripts in the statik blob, which is what actually is distributed. Can you run that & update your PR?

Thanks!

@pbusko pbusko force-pushed the rfc-39-semantic-version-tags branch from f1c6752 to 323b3db Compare November 18, 2021 08:24
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 18, 2021 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@pbusko
Copy link
Contributor Author

pbusko commented Nov 22, 2021

Quick note, you need to run go generate because this PR modifies shell scripts. Running go generate will update the scripts in the statik blob, which is what actually is distributed. Can you run that & update your PR?

Thanks for the quick reply @dmikusa-pivotal! I've updated the PR with the generated files.

@phil9909 phil9909 force-pushed the rfc-39-semantic-version-tags branch 2 times, most recently from 50eb5c4 to f48e942 Compare November 24, 2021 10:11
@phil9909
Copy link
Member

Running go generate will update the scripts in the statik blob, which is what actually is distributed.

I guess its actually go generate ./... or go generate ./octo

@c0d1ngm0nk3y
Copy link
Contributor

@dmikusa-pivotal Could you have another look? I think this is ready to merge (see paketo-buildpacks/github-config#378).

dmikusa
dmikusa requested changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@dmikusa dmikusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also had a diff with octo/statik/statik.go. When I run go generate ./... from the project root. Can you try running that one more time? Also, what is your OS? I'm on a Mac, I wonder if there is some difference there?

Side note: you can run go run cmd/statik/main.go and it'll dump what's in the statik file. If you pipe to a file before and after running go generate you can see what's changed in the file (diff before after).

octo/compute-version.sh Outdated Show resolved Hide resolved
octo/package-buildpack.sh Outdated Show resolved Hide resolved
octo/compute-version.sh Outdated Show resolved Hide resolved
@dmikusa
Copy link
Contributor

dmikusa commented Nov 30, 2021

Sorry, I don't mean to rush, especially after I have delayed on this one (sorry for that), but I wanted to let you know I'll be cutting a release of pipeline builder tomorrow morning US EST. If you're able to address the feedback above, I can merge this in before that release.

Again, no rush. Just wanted to give you a heads up. That would get your change merged in ASAP.

phil9909 and others added 2 commits December 7, 2021 16:11
@phil9909 @pbusko @c0d1ngm0nk3y
Also push MAJOR and MAJOR.MINOR tags when releasing
449edd9 
Co-authored-by: Pavel Busko <pavel.busko@sap.com>
Co-authored-by: Philipp Stehle <philipp.stehle@sap.com>
@pbusko @c0d1ngm0nk3y
push additional tags only during releases
1474120 
Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com>
@pbusko pbusko force-pushed the rfc-39-semantic-version-tags branch from f48e942 to 1474120 Compare December 7, 2021 15:39
@pbusko
Copy link
Contributor Author

pbusko commented Dec 7, 2021

Sorry for the delayed response. We addressed all comments, could you please take another look?

@dmikusa dmikusa added semver:minor A change requiring a minor version bump type:enhancement A general enhancement labels Dec 7, 2021
@dmikusa dmikusa merged commit 8dd8f28 into paketo-buildpacks:main Dec 7, 2021
@pbusko pbusko deleted the rfc-39-semantic-version-tags branch December 7, 2021 20:57
This was referenced Jan 24, 2022
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@buildpack-bot buildpack-bot mentioned this pull request Jan 24, 2022
Merged
@paketo-bot paketo-bot mentioned this pull request Jan 24, 2022
Merged
This was referenced Jan 24, 2022
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@buildpack-bot buildpack-bot mentioned this pull request Jan 24, 2022
Merged
This was referenced Jan 24, 2022
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Feb 8, 2022
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmikusa dmikusa dmikusa approved these changes

Assignees
No one assigned
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pbusko @dmikusa @phil9909 @c0d1ngm0nk3y